Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CI linting with clippy and rustfmt #5

Merged
merged 9 commits into from
Mar 31, 2024

Conversation

nain-F49FF806
Copy link
Contributor

This will automate rust formatting and code quality fixes.

@nain-F49FF806
Copy link
Contributor Author

The second commit here is made automatically by the GitHub action. The first commit defines the CI workflow for the same.
You can see the workflow run here. [workflow run]

@Mydayyy
Copy link
Owner

Mydayyy commented Mar 31, 2024

Greetings,

I hope you are doing well.

Generally I am all in favor of extending the CICD Pipeline here, although I do believe that the CICD Pipeline issuing commits is an antipattern. What do you think about modifying the Pipeline so that it will only check the formatting and fail if the code is not properly formatted? Fixing it would then be up to the user.

Best Regards
Mydayyy

@nain-F49FF806
Copy link
Contributor Author

Absolutely. That can be done.
Just give me a min.

@nain-F49FF806
Copy link
Contributor Author

Greetings,

I have updated to only run the check and not issue automated commits.
Should I also revert the already applied changes? Or do you want to keep them.

Best Regards.

@Mydayyy
Copy link
Owner

Mydayyy commented Mar 31, 2024

Hey,

lets keep those, since I had that on my todo list anyways to let clippy do its clippy things.

Best Regards
Mydayyy

@nain-F49FF806
Copy link
Contributor Author

It should be good to go now. You may have a look.
Cheers.

@nain-F49FF806
Copy link
Contributor Author

Just a min. There's a little thing that needs fixing.
Apologies.

@nain-F49FF806
Copy link
Contributor Author

Greetings,

This should now turn red on any unfixed Clippy warnings. So one can be alerted to fix them.

Best Regards.

@nain-F49FF806
Copy link
Contributor Author

Took care of all uncovered Clippy warnings. 🙃

@Mydayyy Mydayyy merged commit f3ca9a3 into Mydayyy:master Mar 31, 2024
1 check passed
@Mydayyy
Copy link
Owner

Mydayyy commented Mar 31, 2024

Merged, thanks for your contribution!

Best Regards
Mydayyy

@Mydayyy
Copy link
Owner

Mydayyy commented Mar 31, 2024

Did one quick fix on main, didn't catch that in the PR. Replaced main with master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants